Repository navigation
chore(*): gate call correctness with a pinned type checker - #416
Conversation
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
I reviewed the full github/main...HEAD diff and the relevant callers and history for the CI/Makefile gate, every behavior-changing type cleanup, QQ construction, stream bookkeeping, provider resolution, config loading, detached upgrade launch, and the bundled agent/plugin paths. I found no concrete runtime, backward-compatibility, test-integrity, or architecture regression. No tests were weakened; the QQ factory change adds a focused regression test.
Nonblocking follow-up, kept here rather than as an open thread: the explanatory rule counts in [tool.ty] should be reconciled. The block explicitly disables eleven rules, not nine, and an all-rule scan with pinned ty 0.0.80 reports diagnostics in 21 of 133 rule categories, so the stated 124 of 133 baseline is not reproducible from the checked-in configuration. This does not change the gate that CI actually executes.
Verification: the exact frozen command behind lint-types passed; 934 focused tests across all touched behavior areas passed; Ruff check and format, all 10 import contracts, deptry, uv lock --check, source-language validation, and git diff --check passed. The local make lint-types wrapper itself could not be invoked because this environment has no make, so its exact uv recipe was run directly. GitHub's Python lint job is also green; the unit job was still running when this review was posted.
Coverage included AGENTS.md/the linked CLAUDE.md, CONTEXT-MAP.md and the Runtime architecture terms, the diff, callers, relevant history, backward compatibility, test weakening, and architecture boundaries.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
No new findings on this revision. The comment-only delta makes the live-rule arithmetic reproducible: 118 upstream-live rules, minus eleven local disables, plus blanket-ignore-comment, equals 108. It does not alter the configured gate or any runtime behavior. The previously recorded wording follow-up remains nonblocking and is not repeated here.
Verification on this head: the exact frozen ty command passed, all 934 focused tests across the behavior-changing areas passed, and git diff --check passed. GitHub's Python lint, pre-commit, commit-message, repository-file, Windows self-upgrade, kernel-wheel, TUI, UI RPC, page, and bridge checks are green; the unit job was still running when this review was posted.
This round covered the new delta, the full PR diff, the applicable AGENTS.md/linked CLAUDE.md and Runtime context constraints, callers and relevant history from the prior reviewed revision, backward compatibility, test integrity, and architecture boundaries.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
No new findings on this revision. The comment-only delta was checked against pinned ty 0.0.80: its diagnostic totals, mixin/optional breakdown, and cited override/import examples are reproducible, and no rule severity or runtime behavior changed. The previously recorded wording follow-up remains nonblocking and is not repeated here.
Verification on this head: the exact frozen ty gate passed, all 934 focused tests across the behavior-changing areas passed, and git diff --check passed.
This round covered the new delta, the full PR diff, applicable repository and Runtime context rules, the relevant checker output and history, backward compatibility, test integrity, and architecture boundaries. No review threads from this reviewer are open.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
No new findings on this revision. The delta only makes the QQ factory regression test run inside pytest's configured async loop; this removes dependence on ambient thread-loop state while preserving the behavior assertion, and it changes no production path. The existing nonblocking item is unchanged and is not repeated here.
Verification on this head: the exact frozen ty gate passed, all 35 tests in tests/test_channels_qq.py passed, source-language validation passed, and git diff --check passed.
This round covered the new delta, the full PR diff and record, applicable repository and Runtime context rules, the test's caller and history, backward compatibility, test integrity, and architecture boundaries. No review threads from this reviewer are open.
Nothing in this repo could see a signature change reach its callers. Ruff reads one file at a time, the import contracts read edges rather than calls, and the suite only covers lines it executes -- so a required argument added to BoxliteExecutor reached a bundled plugin's render path with CI green, and a reviewer, not a gate, found it. ty answers exactly that question, and 114 of its 133 rules already hold here. Nine are switched off with their measured backlog recorded beside them; the rest are on, which left 21 diagnostics to clear. Of those 21, five were real: - evolver design_of was annotated as taking two parameters while its docstring, its only implementation and its one call site all use three; - _LOGIN_HANDLERS was annotated dict[str, callable] -- the builtin function, not a type, so the annotation asserted nothing; - Config is a BaseSettings but declared env_prefix and env_nested_delimiter through pydantic's ConfigDict, which does not carry those keys; - StreamDelta.conversation_id is optional and was used as a dict[str, str] key: a delta without one stored a None entry that the close path then read as "no open stream", losing the stream's done marker; - a permissive matcher table typed its predicates as object, so nothing checked that they are callable. The rest were unproven invariants or annotations wider than the code. Three late-bound callables are now asserted where a comment used to carry the claim; one closure binds the callable it guarded instead of re-reading the attribute; four hasattr probes became callable(getattr(...)), which is what the call actually needs; the child_scope contextmanager yields the value it just set rather than re-reading it through an optional getter. One suppression is left, with the reason inline: a per-kind spec column whose element shape is not provable without splitting the table. tests/ is out of scope (113 diagnostics under the same rules). Tests are executed, so a broken call there is already loud; this gate is for code that CI does not run. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The block said 124 of 133 rules are on. That counted the fourteen ty ships disabled -- the unsound-* family, division-by-zero, the possibly-* family -- as enabled. They are not: they are unmeasured, which is a different claim from satisfied. Live is 108, and the comment now accounts for all 133. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The counts were right and two of the characterisations were not. The unresolved-attribute note said "mostly third-party without type information"; it is 432 agent-loop mixin attribute reads and 180 optional dereferences, with almost no third-party in it. The invalid-argument-type note asserted a cause that was never sampled. Each entry now says what a reader would find if they opened it, and which ones are worth clearing: invalid-method-override is tool subclasses renaming a base parameter that a model-facing schema passes by keyword, and unresolved-import holds eight stale module paths. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…state botpy.Client.__init__ calls asyncio.get_event_loop(). Outside a running loop that answers from thread-global state: on this machine it warns and creates one, on CI it raised, because whatever ran before had left no current loop. The test passed locally for a reason that was never the test's own. Async instead, so asyncio_mode = "auto" runs the body inside a loop and the call is answered by that loop. Reproduced the CI state first (asyncio.set_event_loop(None), then build the bot -- same RuntimeError), and the async form survives it. Still red when the factory hands back the class. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
36e4b0c to
e4b1741
Compare
The diff gate reported 73% on the changed lines, and it was right: three of the four spots this change touched had no test behind them. child_scope had none at all. It is how the subagent probe hangs its spans under one it does not own, so the contract worth pinning is that the caller's context comes back exactly as it was -- a loan, not an assignment -- and that an entry point with no ambient context still gets a usable one instead of a reparented None. Both go red if the reset is dropped. QQChannel.start() had no test either. The factory has one; this covers the seam where its return becomes self._client and then the object the connect loop awaits, which is the line a wrong return type lands on. The gateway's two broker-wiring sites are inside a 258-line nested function in a 620-line command, and no test reaches serve at all. Rather than leave them uncovered, they now read `callable(getattr(tool := ..., "set_broker", None))` and call `tool.set_broker(...)`, which satisfies the checker the same way while naming the object being wired at the call. main.py's site takes the same shape, so the idiom is written once. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…le on Three of the twenty-one unresolved imports were stale module paths that nothing had ever run into: - evolver's HarnessNode read `raven.__core_version__`, a module that exists neither here nor in the wheel scripts/build_core_wheel.py produces. The read sat inside try/except ImportError, so the except branch was the only branch and every node store.py saved recorded core_version "unknown". The docstring already said where the value comes from -- `raven` version -- so it now reads that, and returns 0.1.13 instead. - raven/agent/loop/wiring.py imported SkillForgeRouterConfig from raven.config.schema; it lives in raven.config.raven. Under TYPE_CHECKING, so nothing raised -- the annotation just meant nothing. - three scripts imported raven.context_engine.base and raven.memory_engine.base, neither of which exists. Two of them failed at module import: running them was not possible. AssemblyContext is in raven.contracts.context and TokenBudget in raven.contracts.assembled. The other eighteen were the environment, not the code. The three agent products keep their plugin package beside them rather than under plugins-dist, so their roots are declared in [tool.ty.environment] the way plugins-dist is declared in ruff's src. lxml ships a compiled module with no type information, so lxml-stubs is a dev dependency now -- real types rather than five silenced lines, though it does surface one stub imprecision (itertext is typed as yielding str or bytes because one element API serves byte-mode trees), suppressed by rule name where it reads. The UNO bridge imports are suppressed per line: that worker runs inside LibreOffice's own interpreter and is never imported by the host. unresolved-import comes off the ignore list, so the gate is 109 of ty's 133 rules. Proven live: an import of a module that does not exist turns `make lint-types` red and removing it turns it green. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Both notes were written from one sample each and both were wrong about the shape of the backlog. invalid-method-override was called the highest-harm family. Enumerated, 53 of its 66 are Tool subclasses naming their parameters against a base that takes only `**kwargs`, which is how a tool publishes its schema; 9 are wrappers that forward through `**kwargs`; 3 rename a parameter together with the schema key it maps to. One is real, and it is a kernel contract question rather than a call site: LLMProvider.chat_stream is declared as a coroutine returning an async iterator while every implementation is an async generator. Turning the rule on would mean suppressing 65 lines to keep that one. invalid-return-type is worth clearing and the note now says what clearing it costs: about half are annotations narrower than the code, half are signatures that refuse a value the function can return. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
invalid-return-type was off with 39 diagnostics. One stale annotation held eight of them: AgentLoop.run_turn and the _run_turn behind it were declared `-> LoopOutcome` while all three of their returns build a TurnOutcome, which is a different dataclass with no field in common. Nothing broke, because the spine worker reads `.usage` and `.explicit_reply` off what it actually gets; LoopOutcome never leaves _run_agent_loop, whose own consumer reads `.status` and `.checkpoint_id`. The annotation was the only thing that was wrong. Three more were annotations narrower than the code: - five `return tuple(<genexpr>)` typed as fixed-arity tuples. `tuple(...)` over a generator is `tuple[T, ...]`, so each now unpacks to the arity it declares. A length that does not match raises where it used to build a wrong-shaped tuple, and in `_as_size` that lands inside the ValueError the caller already catches. - an inner `one()` in the deck reviewer still declared the two-tuple it returned before a third element was added. - image_block returns the ImagePart TypedDict, which is not assignable to a bare dict; the two callers that wrap it say ImagePart now, so the key names are checked at their call sites too. The note in raven/utils/images.py saying CI runs no type checker is no longer true, and no longer says it. Five were declarations refusing a value the function returns: four tool `execute` overrides said `-> str` while returning ToolResult on their error paths (the base is `str | ToolResult`), and a sentinel helper said `MenuReply` while returning None on two paths its only caller already types as optional. Seven were invariants that lived in a trailing `# type: ignore` comment -- the chain is never empty, _do_refresh sets it or raises, the constructor refuses an empty key. Those are asserts now, stated where both a reader and the checker meet them, each with the noqa that says why an assert is the right shape. The rest are honest widenings: a shaping dict that really does carry a None, a delivered path that really can be absent, an api-key source that really is `str | None`, and an onboarding prompt that really can return the untyped `_BACK` sentinel -- the same `Any` its four siblings already declare. Giving that sentinel a type of its own is a separate change across twelve sites. The gate is now 110 of ty's 133 rules. Proven live: a function returning a str under `-> int` turns `make lint-types` red, and removing it turns it green. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
No new findings on this revision. The rebase onto the sharded-test main is clean, the added QQ and tracing tests cover the new behavior, and restoring the broker call expressions to their pre-existing form leaves only the stricter callable guards as the production delta. The existing nonblocking item is unchanged and is not repeated here.
Verification on this head: the exact frozen ty gate passed; 189 targeted QQ, tracing, deep-research, and gateway tests passed; Ruff on the touched delta, source-language validation, and git diff --check passed. GitHub's four unit shards, combined coverage gate, Python lint, pre-commit, commit checks, Windows upgrade, kernel wheel, TUI, UI RPC, page, and bridge checks are all green.
This round covered the rebase range-diff, new commit and full PR diff, applicable repository and Runtime context rules, callers and history, backward compatibility, test integrity, coverage accounting, and architecture boundaries. No review threads from this reviewer are open.
…h routes The two spots the diff gate named were the asserts this change had just added, and they were the wrong shape twice over. _do_refresh sets self._data and returns None, so the caller had to read the attribute back and claim it was not None -- and the claim sat inside the try, where an AssertionError would have been swallowed by the `except Exception` below it and served the stale copy instead of failing. _do_refresh returns the data it fetched now, so the caller returns that. Two asserts, two noqa and the swallowing all go with it. BenchmarkCache had no test. Its two refresh-success routes decide whether a router sees today's numbers or yesterday's, so they are covered now, with the failed-refresh route beside them as the contrast that says why the disk copy is a fallback and not the answer. The stale-route test goes red if the refresh is skipped. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the new d5f0588f82eb...f64bff7be1db delta and rechecked the full github/main...HEAD change against AGENTS.md/CLAUDE.md and the CONTEXT architecture contracts, including callers and history, backward compatibility, and whether tests were weakened. The two newly enabled rules (unresolved-import and invalid-return-type) are cleared with corrections that preserve the runtime contracts; in particular, _do_refresh() now returns its fetched value directly to both foreground callers while its background caller safely ignores that value. The gate accounting now matches the configuration at 110 live rules and nine locally disabled rules. The delta only adds tests rather than weakening existing coverage.
Verification: the exact pinned ty gate passed locally; 519 focused routing, evolver, loop, CLI, QQ, and tracing tests passed; and Ruff passed on the newly changed implementation/tests. GitHub's four unit shards, coverage gates, Python lint/type checks, kernel-wheel smoke, repository checks, TUI/page/RPC checks, and Windows upgrade check all pass. The red Bridge job is unrelated to this diff: its TypeScript build passed, then npm's retiring audit endpoint returned 400 Invalid package tree; bridge sources and lockfile are unchanged. A local bridge build could not start because this checkout has no bridge dependencies installed (tsc was absent), so the successful CI compile is the meaningful build result. No review threads are open.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
This revision only merges the current target branch. I checked the refreshed github/main...HEAD diff, the merge against both parents, commit history/range-diff, and the stable patch ID: all nine PR commits and the complete PR patch are unchanged, with no conflict-resolution delta. That also preserves the prior conclusions on callers, backward compatibility, test strength, AGENTS.md/CLAUDE.md rules, and the CONTEXT architecture contracts.
On the merged tree, the exact pinned ty gate passed and the focused routing, evolver, loop, CLI, QQ, and tracing run passed all 519 tests. CI's lint, repository, commit, bridge, kernel-wheel, page, TUI, RPC, Windows, and three unit-shard checks pass. The remaining unit shard ran all 5,429 tests successfully but failed the new idle ceiling because an upstream tests/test_rpc_model.py case recorded 3.44 seconds idle against a 3-second limit. This PR changes neither that test nor the idle-ceiling machinery, the test passes locally with --idle-ceiling-strict, and the target commit itself already has a failing unit shard; this is not a regression introduced by this revision. No review threads are open.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; suggestions only, and they are marked inline.
This revision is another merge of main. I checked both parents, the refreshed github/main...HEAD diff, history/range-diff, and stable patch IDs: all nine PR commits and the full PR patch remain identical, with no conflict-resolution change. The prior review of repository rules, callers, compatibility, architecture contracts, and test strength therefore still applies; the new work here was checking interaction with the updated target.
Named nonblocking follow-up (summary-only because the affected line is untouched target code): the pinned gate now reports one unresolved-import in raven/rpc/knowledge_preview.py:34. KnowledgeDocumentRecord is imported from raven.knowledge._types, but it is defined in _records and exported from raven.knowledge. The line came from target commit 7beffb9d0f, not this PR, so this is a pre-existing issue newly exposed by the gate and does not meet the late-round blocker requirement that this change introduce the defect. Correcting that import on main or carrying the same correction here will clear the currently red Python-lint job.
Verification: the exact ty command fails only on that import; the focused merged-tree run passed all 519 tests; all four CI unit shards, coverage, and every other reported job pass. No tests were weakened and no review threads are open.
`knowledge_preview` imported KnowledgeDocumentRecord from `raven.knowledge._types`, which does not define it; it lives in `raven.knowledge._records`, and that is what `manager.get_document()` hands back. The import sits under TYPE_CHECKING, so nothing raised -- the five annotations it feeds, on `resolve`, `named`, `is_renderable`, `pdf_for` and `_alias_for`, simply resolved to nothing. Arrived from the target branch and caught by this PR's own gate, which is the same shape it already cleared in `agent/loop/wiring.py`: a TYPE_CHECKING import naming the wrong module is invisible until something reads the annotation. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The new one-line delta resolves the target-branch interaction noted on the previous revision: KnowledgeDocumentRecord is defined in raven.knowledge._records, and that is the class returned by KnowledgeManager.get_document(). Because the import is under TYPE_CHECKING, the correction changes annotation resolution only and preserves runtime/backward compatibility. I checked the refreshed full diff, the symbol's definition and caller, commit history, AGENTS.md/CLAUDE.md and CONTEXT constraints, and test changes; no tests were weakened.
Verification: the exact pinned ty gate now passes, 49 knowledge-preview/routing/evolver tests pass locally, and the revision's Python lint/type-check CI job plus all completed non-unit jobs are green. The remaining unit shards were still running when reviewed. No review threads are open.
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
This revision is another merge of current main into the branch. The range-diff preserves all 10 PR commits one-for-one, and the stable patch ID is unchanged from the clean 891316dd7c58 revision. I checked the repository rules, the full PR diff and new-main overlap, affected callers and history, backward compatibility, test-strength changes, and the existing architecture constraints; the merge introduces no new PR-side behavior or weakened tests.
Verification:
uv run --frozen --python 3.12 --all-extras ty check raven evolver agents plugins-dist scripts- passeduv run pytest tests/test_routing_benchmark_cache.py tests/test_evolver_node_core_version.py tests/test_agent_loop_run_emit.py tests/test_cli_session_commands.py tests/test_cli_onboard_commands.py tests/test_channels_qq.py tests/test_tracing_api.py -q- 529 passedgit diff --check github/main...HEAD- passed
All completed GitHub checks were green when inspected; the four unit shards were still pending.
…ads (#463) ## Summary `main` is red on three jobs (`python lint`, `unit 2/4`, `unit 3/4`), and has been since #451 merged. All of it is fallout from one arrival: `image_search` came with a second switch beside its credential. `tools.web.search.images` decides whether the tool is registered at all (`raven/agent/loop/wiring.py`), so a lane that never places a picture keeps the tool face it had. The capability table was not told. `is_configured` answers on the vendor key alone, so with a Serper key and the switch left off the table ticked a capability the agent does not hold, and the two tests that drive a real `AgentLoop` caught exactly the disagreement that module exists to prevent. The switch belongs on `is_disabled`, not `is_configured`. That function's own docstring already names the reason: a switched-off tool usually has its credential set, and calling it unconfigured sends the deployer to set a key that is already there. `tools.disabledTools` and this flag are the same kind of answer, so `is_offered` needs no change and the doctor row reads "off" rather than "missing a key". The test configs that mean "everything supplied" now turn the switch on, the way `raven/core/runtime.py` passes it from the config, and a new case pins the combination that was wrong: key present, pictures off, nothing offered, table saying off. Two more pieces of the same arrival: - `ImageSearchTool` copied `WebSearchTool`'s key resolution without its two attribute annotations, so the untyped lambda `wiring.py` hands it left the `api_key` property returning `~AlwaysFalsy | str` against a declared `str`. The type checker pinned in #416 reports it; the annotations are the fix. - `WebSearchConfig` gained the `images` field, so the writer's validated subtree now carries it and two `test_config_update_tools` expectations that pin the written section by value went stale. Not changed here: `get_web_search` still reports `{provider, api_key, max_results}` and does not surface `images`, though `set_web_search` can write it. That is a gap in the read side of the config API, not a CI failure, and it wants its own change. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification Run on Windows 11 in a clean worktree cut from `main` (`5a29ea22`): - `uv run pytest tests/test_tool_capabilities.py tests/test_config_update_tools.py -q`: 71 passed. All four tests failing on `main` now pass, plus the new case. - `uv run --frozen --python 3.12 --all-extras ty check raven evolver agents plugins-dist scripts`: the `web.py:516` diagnostic is gone. The diagnostics that remain locally are unresolved imports for optional packages absent from this machine's environment (`boxlite`, `nio`); CI reported exactly one diagnostic before this change, the one fixed here. - `uv run --frozen --python 3.12 --extra dev ruff check raven tests`: all checks passed. - `ruff format --check` on the four changed files: already formatted. - `uv run pytest tests/test_cli_doctor_commands.py tests/test_rpc_console.py -q`, which consume `is_disabled`: the failing-test set is byte-identical before and after this change (8 pre-existing Windows-only failures around file modes and an absent browser package, none of them in the set CI reports). ## Risk `is_disabled` gains a second reason to answer true, so a deployment with a search key and `tools.web.search.images` off now reads as "switched off" in `raven doctor` and the console capability report, where it previously read as offered. That is the state the loop was already in; only the description changes. Nothing about registration, gating, or the tool's runtime behaviour moves. The other two changes are an annotation and two test expectations, with no runtime effect. Rollback is reverting this commit; it touches nothing that other work builds on. ## Related Issues N/A --------- Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
## Summary Nothing in this repo could see a signature change reach its callers. Ruff reads one file at a time, the import contracts read edges rather than calls, and the suite only covers lines it executes. The consequence was concrete: a required `sandbox_home` argument added to `BoxliteExecutor` reached `plugins-dist/design-engine`'s render path with CI green, and a reviewer -- not a gate -- found it. `ty` answers that question. This adds it as a pinned dev dependency, a `make lint-types` target and a CI step, with **110 of its 133 rules live**: nine switched off with their measured backlog recorded beside each, fourteen shipped disabled upstream and untouched. Getting there meant clearing 81 diagnostics. This PR folds what were three stacked PRs (#422 and #423, now closed); each rule's story is a commit of its own. ### 1. The gate, at 108 rules (`chore(*): gate call correctness...`) 21 diagnostics to clear, five of them real: | Where | What | |---|---| | `evolver/orchestrator/production.py` | `design_of` annotated as taking two parameters while its docstring, its only implementation and its one call site all use three | | `raven/cli/provider_commands.py` | `dict[str, callable]` -- the builtin function, not a type, so the annotation asserted nothing | | `raven/config/schema.py` | `Config` is a `BaseSettings` but declared `env_prefix` / `env_nested_delimiter` through pydantic's `ConfigDict`, which does not carry those keys | | `raven/spine/delivery.py` | `StreamDelta.conversation_id` is optional and was used as a `dict[str, str]` key; a delta without one stored a `None` entry the close path then read as "no open stream", losing that stream's done marker | | `research_flow/support/harness_text.py` | a predicate table typed its matchers as `object`, so nothing checked they are callable | The rest were unproven invariants and annotations wider than the code: three late-bound callables asserted where a comment used to carry the claim, one closure binding the callable it guarded instead of re-reading it, four `hasattr` probes that now check what the call actually needs, and a contextmanager yielding the value it just set. ### 2. `unresolved-import` (`fix(*): resolve the imports the type gate could not`) 21 diagnostics; three were stale module paths nothing had run into: - `HarnessNode.current_core_version()` read `raven.__core_version__`, a module that exists neither here nor in the wheel `scripts/build_core_wheel.py` produces. The read sat inside `try/except ImportError`, so the except branch was the only branch and **every node `store.py` saved recorded `core_version="unknown"`**. It reads `raven.__version__` now and returns `0.1.13`. - `raven/agent/loop/wiring.py` imported `SkillForgeRouterConfig` from `raven.config.schema`; it lives in `raven.config.raven`. Under `TYPE_CHECKING`, so the annotation simply meant nothing. - three scripts imported `raven.context_engine.base` and `raven.memory_engine.base`, neither of which exists. **Two failed at module import** -- running them was not possible. The other eighteen were the environment: the three agent products keep their plugin package beside them, so their roots are declared in `[tool.ty.environment]`; `lxml` ships a compiled `etree` with no type information, so `lxml-stubs` is a dev dependency (real types rather than five silenced lines); `uno` and `com.sun.star.*` are suppressed per line, since that worker runs inside LibreOffice's own interpreter. ### 3. `invalid-return-type` (`fix(*): make the declared return types true`) 39 diagnostics, and **one stale annotation held eight of them**: `run_turn` and the `_run_turn` behind it were declared `-> LoopOutcome` while all three of their returns build a `TurnOutcome` -- a different dataclass with no field in common. Nothing broke, because the spine worker reads what it actually gets and `LoopOutcome` never leaves `_run_agent_loop`. The annotation was the only thing wrong. The other 31: nine annotations narrower than the code (five `return tuple(<genexpr>)` typed as fixed-arity tuples, an inner `one()` still declaring the two-tuple it returned before a third element was added, `image_block`'s TypedDict), five declarations refusing a value the function returns (four tool `execute` overrides saying `-> str` while returning `ToolResult`), seven invariants that lived in a `# type: ignore` comment, and ten honest widenings. ### 4. What the gates then caught in this PR's own work - The qq factory guard passed locally only because an earlier test had left an event loop behind. Reproduced the CI state (`asyncio.set_event_loop(None)`), then made the test async so the loop is its own. - `invalid-return-type`, while still disabled, hid a regression introduced here: the qq factory returning the class instead of an instance. Found by diffing the rule histogram, which is why that rule is on now. - The diff-coverage gate named four lines in `raven/routing/cache.py` -- the asserts this work had just added, sitting inside a `try` whose `except Exception` would have swallowed the `AssertionError` and served the stale cache. `_do_refresh` returns its data now; the asserts, their noqa and the swallowing all go with it. `child_scope`, `QQChannel.start()` and `BenchmarkCache` had no tests at all and have them now. Every one was mutation-checked. ## Type - [ ] Fix - [ ] Feature - [ ] Docs - [x] CI / tooling - [ ] Refactor - [ ] Other ## Verification - `make lint-types` -> All checks passed; 81 diagnostics cleared across the three rule sets - Gate proven live three times, once per rule: a missing argument, an import of a module that does not exist, and a function returning a str under `-> int` each turn it red, and removing the probe turns it green - `uv run ruff check` + `ruff format --check` over all six trees -> clean - `uv run lint-imports` -> 10 kept, 0 broken; `deptry raven` (all extras) -> no issues; `uv lock --check` -> resolved - `make check-source-language` -> exit 0; `commitlint` -> exit 0 - `make coverage` locally: **22050 passed, 45 skipped, 4 failed**; diff coverage 92.59% - All four failures are **pre-existing on `main`**, checked by a real checkout in the same environment where they fail identically: two `test_the_products_tool_face_*` launcher tests, `test_a_ceiling_as_large_as_the_window_still_leaves_room_for_history` (`reserved_tools = 6044 > 6_000`), and `test_a_missing_key_is_said_once_rather_than_per_query` (environment-dependent: it asserts `SERPER_API_KEY` is absent) - Rule counts measured with both `--extra dev` and `--all-extras` -- identical, so this gate's verdict does not depend on which optional packages are installed - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed ## Risk Behaviour changes are confined to the fixes listed above, and each replaces an error path with a correct one: nodes are stamped with the real raven version instead of `"unknown"`, two scripts that could not start now start, the stream bookkeeping no longer stores a `None` key, and a cache refresh returns what it fetched. Everything else is a declaration moved to match what the function already returned. `tests/` is out of the gate's scope (113 diagnostics under the same rules). Tests are executed, so a broken call there is already loud; this gate is for code CI does not run. Bringing them in is sized, separate work. Rollback: revert the whole PR to drop the gate and its fixes together, or delete the `[tool.ty]` block, the `lint-types` target and its CI step to keep the fixes and drop only the gate. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues N/A --------- Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…ads (#463) ## Summary `main` is red on three jobs (`python lint`, `unit 2/4`, `unit 3/4`), and has been since #451 merged. All of it is fallout from one arrival: `image_search` came with a second switch beside its credential. `tools.web.search.images` decides whether the tool is registered at all (`raven/agent/loop/wiring.py`), so a lane that never places a picture keeps the tool face it had. The capability table was not told. `is_configured` answers on the vendor key alone, so with a Serper key and the switch left off the table ticked a capability the agent does not hold, and the two tests that drive a real `AgentLoop` caught exactly the disagreement that module exists to prevent. The switch belongs on `is_disabled`, not `is_configured`. That function's own docstring already names the reason: a switched-off tool usually has its credential set, and calling it unconfigured sends the deployer to set a key that is already there. `tools.disabledTools` and this flag are the same kind of answer, so `is_offered` needs no change and the doctor row reads "off" rather than "missing a key". The test configs that mean "everything supplied" now turn the switch on, the way `raven/core/runtime.py` passes it from the config, and a new case pins the combination that was wrong: key present, pictures off, nothing offered, table saying off. Two more pieces of the same arrival: - `ImageSearchTool` copied `WebSearchTool`'s key resolution without its two attribute annotations, so the untyped lambda `wiring.py` hands it left the `api_key` property returning `~AlwaysFalsy | str` against a declared `str`. The type checker pinned in #416 reports it; the annotations are the fix. - `WebSearchConfig` gained the `images` field, so the writer's validated subtree now carries it and two `test_config_update_tools` expectations that pin the written section by value went stale. Not changed here: `get_web_search` still reports `{provider, api_key, max_results}` and does not surface `images`, though `set_web_search` can write it. That is a gap in the read side of the config API, not a CI failure, and it wants its own change. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification Run on Windows 11 in a clean worktree cut from `main` (`5a29ea22`): - `uv run pytest tests/test_tool_capabilities.py tests/test_config_update_tools.py -q`: 71 passed. All four tests failing on `main` now pass, plus the new case. - `uv run --frozen --python 3.12 --all-extras ty check raven evolver agents plugins-dist scripts`: the `web.py:516` diagnostic is gone. The diagnostics that remain locally are unresolved imports for optional packages absent from this machine's environment (`boxlite`, `nio`); CI reported exactly one diagnostic before this change, the one fixed here. - `uv run --frozen --python 3.12 --extra dev ruff check raven tests`: all checks passed. - `ruff format --check` on the four changed files: already formatted. - `uv run pytest tests/test_cli_doctor_commands.py tests/test_rpc_console.py -q`, which consume `is_disabled`: the failing-test set is byte-identical before and after this change (8 pre-existing Windows-only failures around file modes and an absent browser package, none of them in the set CI reports). ## Risk `is_disabled` gains a second reason to answer true, so a deployment with a search key and `tools.web.search.images` off now reads as "switched off" in `raven doctor` and the console capability report, where it previously read as offered. That is the state the loop was already in; only the description changes. Nothing about registration, gating, or the tool's runtime behaviour moves. The other two changes are an annotation and two test expectations, with no runtime effect. Rollback is reverting this commit; it touches nothing that other work builds on. ## Related Issues N/A --------- Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Summary
Nothing in this repo could see a signature change reach its callers. Ruff reads one file at a time, the import contracts read edges rather than calls, and the suite only covers lines it executes. The consequence was concrete: a required
sandbox_homeargument added toBoxliteExecutorreachedplugins-dist/design-engine's render path with CI green, and a reviewer -- not a gate -- found it.tyanswers that question. This adds it as a pinned dev dependency, amake lint-typestarget and a CI step, with 110 of its 133 rules live: nine switched off with their measured backlog recorded beside each, fourteen shipped disabled upstream and untouched.Getting there meant clearing 81 diagnostics. This PR folds what were three stacked PRs (#422 and #423, now closed); each rule's story is a commit of its own.
1. The gate, at 108 rules (
chore(*): gate call correctness...)21 diagnostics to clear, five of them real:
evolver/orchestrator/production.pydesign_ofannotated as taking two parameters while its docstring, its only implementation and its one call site all use threeraven/cli/provider_commands.pydict[str, callable]-- the builtin function, not a type, so the annotation asserted nothingraven/config/schema.pyConfigis aBaseSettingsbut declaredenv_prefix/env_nested_delimiterthrough pydantic'sConfigDict, which does not carry those keysraven/spine/delivery.pyStreamDelta.conversation_idis optional and was used as adict[str, str]key; a delta without one stored aNoneentry the close path then read as "no open stream", losing that stream's done markerresearch_flow/support/harness_text.pyobject, so nothing checked they are callableThe rest were unproven invariants and annotations wider than the code: three late-bound callables asserted where a comment used to carry the claim, one closure binding the callable it guarded instead of re-reading it, four
hasattrprobes that now check what the call actually needs, and a contextmanager yielding the value it just set.2.
unresolved-import(fix(*): resolve the imports the type gate could not)21 diagnostics; three were stale module paths nothing had run into:
HarnessNode.current_core_version()readraven.__core_version__, a module that exists neither here nor in the wheelscripts/build_core_wheel.pyproduces. The read sat insidetry/except ImportError, so the except branch was the only branch and every nodestore.pysaved recordedcore_version="unknown". It readsraven.__version__now and returns0.1.13.raven/agent/loop/wiring.pyimportedSkillForgeRouterConfigfromraven.config.schema; it lives inraven.config.raven. UnderTYPE_CHECKING, so the annotation simply meant nothing.raven.context_engine.baseandraven.memory_engine.base, neither of which exists. Two failed at module import -- running them was not possible.The other eighteen were the environment: the three agent products keep their plugin package beside them, so their roots are declared in
[tool.ty.environment];lxmlships a compiledetreewith no type information, solxml-stubsis a dev dependency (real types rather than five silenced lines);unoandcom.sun.star.*are suppressed per line, since that worker runs inside LibreOffice's own interpreter.3.
invalid-return-type(fix(*): make the declared return types true)39 diagnostics, and one stale annotation held eight of them:
run_turnand the_run_turnbehind it were declared-> LoopOutcomewhile all three of their returns build aTurnOutcome-- a different dataclass with no field in common. Nothing broke, because the spine worker reads what it actually gets andLoopOutcomenever leaves_run_agent_loop. The annotation was the only thing wrong.The other 31: nine annotations narrower than the code (five
return tuple(<genexpr>)typed as fixed-arity tuples, an innerone()still declaring the two-tuple it returned before a third element was added,image_block's TypedDict), five declarations refusing a value the function returns (four toolexecuteoverrides saying-> strwhile returningToolResult), seven invariants that lived in a# type: ignorecomment, and ten honest widenings.4. What the gates then caught in this PR's own work
asyncio.set_event_loop(None)), then made the test async so the loop is its own.invalid-return-type, while still disabled, hid a regression introduced here: the qq factory returning the class instead of an instance. Found by diffing the rule histogram, which is why that rule is on now.raven/routing/cache.py-- the asserts this work had just added, sitting inside atrywhoseexcept Exceptionwould have swallowed theAssertionErrorand served the stale cache._do_refreshreturns its data now; the asserts, their noqa and the swallowing all go with it.child_scope,QQChannel.start()andBenchmarkCachehad no tests at all and have them now. Every one was mutation-checked.Type
Verification
make lint-types-> All checks passed; 81 diagnostics cleared across the three rule setsGate proven live three times, once per rule: a missing argument, an import of a module that does not exist, and a function returning a str under
-> inteach turn it red, and removing the probe turns it greenuv run ruff check+ruff format --checkover all six trees -> cleanuv run lint-imports-> 10 kept, 0 broken;deptry raven(all extras) -> no issues;uv lock --check-> resolvedmake check-source-language-> exit 0;commitlint-> exit 0make coveragelocally: 22050 passed, 45 skipped, 4 failed; diff coverage 92.59%All four failures are pre-existing on
main, checked by a real checkout in the same environment where they fail identically: twotest_the_products_tool_face_*launcher tests,test_a_ceiling_as_large_as_the_window_still_leaves_room_for_history(reserved_tools = 6044 > 6_000), andtest_a_missing_key_is_said_once_rather_than_per_query(environment-dependent: it assertsSERPER_API_KEYis absent)Rule counts measured with both
--extra devand--all-extras-- identical, so this gate's verdict does not depend on which optional packages are installedRelevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
Behaviour changes are confined to the fixes listed above, and each replaces an error path with a correct one: nodes are stamped with the real raven version instead of
"unknown", two scripts that could not start now start, the stream bookkeeping no longer stores aNonekey, and a cache refresh returns what it fetched. Everything else is a declaration moved to match what the function already returned.tests/is out of the gate's scope (113 diagnostics under the same rules). Tests are executed, so a broken call there is already loud; this gate is for code CI does not run. Bringing them in is sized, separate work.Rollback: revert the whole PR to drop the gate and its fixes together, or delete the
[tool.ty]block, thelint-typestarget and its CI step to keep the fixes and drop only the gate.Related Issues
N/A